Skip to content

Conversation

vvnsrzn
Copy link
Contributor

@vvnsrzn vvnsrzn commented Aug 4, 2025

Hello @coyotte508

I stumbled upon this issue.

I suggest to use the set method instead.

Benchmark: https://jsben.ch/jze3P

@vvnsrzn vvnsrzn requested a review from coyotte508 as a code owner August 4, 2025 12:12
@vvnsrzn vvnsrzn changed the title perf: uint8array set method instead of spread ⚡️ uint8array set method instead of spread Aug 4, 2025
@vvnsrzn vvnsrzn force-pushed the perf-xet-fetchdata branch from 38b42c8 to 028f861 Compare August 8, 2025 09:18
@@ -327,8 +327,11 @@ export class XetBlob extends Blob {
totalFetchBytes += result.value.byteLength;

if (leftoverBytes) {
result.value = new Uint8Array([...leftoverBytes, ...result.value]);
leftoverBytes = undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the leftoverBytes = undefined part is important

Copy link
Member

@coyotte508 coyotte508 Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also make the function into a combineUint8Arrays helper? ( a dedicated file in this folder)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, I'll take care of it.

@vvnsrzn vvnsrzn force-pushed the perf-xet-fetchdata branch 2 times, most recently from 7a7e64f to 699b51a Compare August 30, 2025 12:26
@vvnsrzn vvnsrzn requested a review from coyotte508 August 30, 2025 12:27
@coyotte508
Copy link
Member

some formatting issues, you can run pnpm -r format or pnpm --filter hub format

@vvnsrzn
Copy link
Contributor Author

vvnsrzn commented Sep 1, 2025

some formatting issues, you can run pnpm -r format or pnpm --filter hub format

Oh sorry for that, I didn't run the right command.

Copy link
Member

@coyotte508 coyotte508 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@coyotte508 coyotte508 merged commit 4f037c1 into huggingface:main Sep 1, 2025
3 of 4 checks passed
@vvnsrzn vvnsrzn deleted the perf-xet-fetchdata branch September 1, 2025 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants